Optimizer misc improvements - #26704
Conversation
|
Benchmarks started. Workflow run. |
|
Benchmarks completed. Overview. |
|
|
| private val modulesAllowSkipInitialization = | ||
| if settings.optAllowSkipCoreModuleInit then optimizerUtils.modulesAllowSkipInitialization else Set.empty | ||
| private val modulesAllowSkipInitialization: InternalName => Boolean = | ||
| if settings.optAllowSkipCoreModuleInit then n => n.startsWith("scala/") else Set.empty |
There was a problem hiding this comment.
I'd personally still go for a whitelist.. we don't know what will be added to the library in the future, and there can be user-defined code in a scala package.
There was a problem hiding this comment.
OK. Although user-defined code in the scala package isn't a supported scenario AFAIK (e.g., Seb pointed out that it would in theory prevent us from ever adding new stdlib classes since they could conflict)
| // invokespecial has, well, special semantics that depend on the class it's being invoked in, see, e.g., https://stackoverflow.com/a/8950564 | ||
| if (!allowPrivateCalls && i.getOpcode == Opcodes.INVOKESPECIAL && mi.name != BCodeUtils.INSTANCE_CONSTRUCTOR_NAME) { | ||
| if (mi.owner == ScalaPackage) { | ||
| // Ignore calls that, e.g., load the Range module -- that's still part of a forwarder |
There was a problem hiding this comment.
This also seems too broad. Also, how is the Scala package object involved in loading the Range module?
There was a problem hiding this comment.
I'm not sure how new this is, but you can load modules from the package object now. This is what it contains on main:
solal@LAMP-C007400M scala3 % javap ./library/target/scala-library-bootstrapped/classes/scala/package$.class
Compiled from "package.scala"
public final class scala.package$ {
public static final scala.package$ MODULE$;
public static {};
public scala.Specializable AnyRef();
public scala.collection.Iterable$ Traversable();
public scala.collection.Iterable$ Iterable();
public scala.collection.immutable.Seq$ Seq();
public scala.collection.immutable.IndexedSeq$ IndexedSeq();
public scala.collection.Iterator$ Iterator();
public scala.collection.immutable.List$ List();
public scala.collection.immutable.Nil$ Nil();
public scala.collection.immutable.$colon$colon$ $colon$colon();
public scala.collection.package$$plus$colon$ $plus$colon();
public scala.collection.package$$colon$plus$ $colon$plus();
public scala.collection.immutable.Stream$ Stream();
public scala.collection.immutable.LazyList$ LazyList();
public scala.collection.immutable.Vector$ Vector();
public scala.collection.mutable.StringBuilder$ StringBuilder();
public scala.collection.immutable.Range$ Range();
public scala.math.BigDecimal$ BigDecimal();
public scala.math.BigInt$ BigInt();
public scala.math.Equiv$ Equiv();
public scala.math.Fractional$ Fractional();
public scala.math.Integral$ Integral();
public scala.math.Numeric$ Numeric();
public scala.math.Ordered$ Ordered();
public scala.math.Ordering$ Ordering();
public scala.util.Either$ Either();
public scala.util.Left$ Left();
public scala.util.Right$ Right();
}There was a problem hiding this comment.
I added a check that it's within the side-effect-free whitelist
Co-authored-by: Lukas Rytz <lukas.rytz@gmail.com>
5d4d38e to
3613634
Compare
3 participants
In support of the
bouncebenchmark (source):scala/package$(non-null, no side effects)applyVoidas a specialization ofapplyfor functionsscala/*With this, we can inline all
foreachcalls and we don't need anIntRefHave you relied on LLM-based tools in this contribution?
No
How was the solution tested?
New automated tests (including the issue's reproducer, if applicable)